Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow for return partials with falsy arguments #9298

Merged
merged 2 commits into from
Dec 17, 2021
Merged

Conversation

ptgott
Copy link
Contributor

@ptgott ptgott commented Dec 16, 2021

Partials with returns values are parsed, then inserted into a
partial return wrapper via wrapInPartialReturnWrapper in order
to assign the return value via *contextWrapper.Set. The
predefined wrapper template for partials inserts a partial's nodes
into a "with" template action in order to set dot to a
*contextWrapper within the partial. However, because "with" is
skipped if its argument is falsy, partials with falsy arguments
were not being evaluated.

This replaces the "with" action in the partial wrapper with a
"range" action that isn't skipped if .Arg is falsy.

Fixes #7528

@ptgott
Copy link
Contributor Author

ptgott commented Dec 16, 2021

mage -v test failed on my local branch with tests that also fail in origin/master, so fingers crossed for the CI pipeline.

@bep
Copy link
Member

bep commented Dec 16, 2021

Thanks for this, I thought long and hard about this problem ... never thought about using the range keyword. I will do some tests on my own tomorrow, but it it works, I'll merge it. Thanks!

@@ -112,7 +112,7 @@ func getParseTree(templ tpl.Template) *parse.Tree {
}

const (
partialReturnWrapperTempl = `{{ $_hugo_dot := $ }}{{ $ := .Arg }}{{ with .Arg }}{{ $_hugo_dot.Set ("PLACEHOLDER") }}{{ end }}`
partialReturnWrapperTempl = `{{ $_hugo_dot := $ }}{{ $ := .Arg }}{{ range (slice .Arg) }}{{ $_hugo_dot.Set ("PLACEHOLDER") }}{{ end }}`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect it would be easier to understand if you make contextWrapper.Arg a []interface{} -- and just add a comment there explaining why it's a slice with a single value.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I gave this a go, though my attempt at a comment depended on explaining what we were doing with the partialReturnWrapperTempl. In the end, I found it simpler to add a comment above partialReturnWrapperTempl while leaving contextWrapper.Arg as it was. How does that sound?

Partials with returns values are parsed, then inserted into a
partial return wrapper via wrapInPartialReturnWrapper in order
to assign the return value via *contextWrapper.Set. The
predefined wrapper template for partials inserts a partial's nodes
into a "with" template action in order to set dot to a
*contextWrapper within the partial. However, because "with" is
skipped if its argument is falsy, partials with falsy arguments
were not being evaluated.

This replaces the "with" action in the partial wrapper with a
"range" action that isn't skipped if .Arg is falsy.

Fixes gohugoio#7528
@bep bep merged commit 5758c37 into gohugoio:master Dec 17, 2021
@ptgott ptgott deleted the issue-7528 branch December 17, 2021 14:24
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

No proper return from partials with 0 as arg
2 participants